Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes #1819

Merged
merged 5 commits into from
Jun 1, 2016
Merged

Bug fixes #1819

merged 5 commits into from
Jun 1, 2016

Conversation

kaadhina
Copy link
Contributor

@kaadhina kaadhina commented Jun 1, 2016

Fixing
BUG 592101: Minor help text fix
BUG 594989: Supporting publish from custom results location in runsettings
Validation: Manual , UTs


# If this is a runsettings file then try to get the custom results location from it

if((([string]::Compare([io.path]::GetExtension($runSettingsFilePath), ".runsettings", $True) -eq 0) -Or ([string]::Compare([io.path]::GetExtension($runSettingsFilePath), ".tmp", $True) -eq 0)) -And
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can u simplify this? Refactor the into method like
IsRunSettingsFile
CheckIfNotFolder etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@acesiddhu
Copy link
Contributor

rest looks fine to me

if((([string]::Compare([io.path]::GetExtension($runSettingsFilePath), ".runsettings", $True) -eq 0) -Or ([string]::Compare([io.path]::GetExtension($runSettingsFilePath), ".tmp", $True) -eq 0)) -And
!(Test-Path $runSettingsFilePath -pathtype container))
{
$runSettingsForTestResults = [System.Xml.XmlDocument](Get-Content $runSettingsFilePath)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens in case of malformed XML?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the node we are looking for is not there, it will default to old behaviour. If the xml structure itself is wrong, it will fail as expected with appropriate error.

@nigurr
Copy link

nigurr commented Jun 1, 2016

Approving with suggestions. Please do resolve them

@kaadhina kaadhina merged commit f9f4c6c into master Jun 1, 2016
@kaadhina kaadhina deleted the users/kaadhina/customresults branch June 1, 2016 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants